-
Notifications
You must be signed in to change notification settings - Fork 68
[PLT-0] LBOX Namespace, Plugin Style SDK #1656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f24fc7c
to
b12032b
Compare
c277945
to
c66fbb6
Compare
e62ef86
to
d938d4b
Compare
I've reviewed the wonderful wiki documentation [Wiki Link] and skimmed through the pull request related to the proposed changes for the SDK. Based on my understanding it seems the proposal aims to address several objectives:
Complexity Concerns: While I appreciate the potential benefits of these changes, I'm concerned that the proposed solution might lead to an increase in overall SDK development complexity in the near future. Personally, I would prefer to minimize such complexity. Before moving forward with this specific proposal, I believe it would be valuable to take a step back and identify the most pressing challenges we need to address for the SDK's future development. By focusing on the core problems, we can potentially explore alternative solutions that achieve our goals without introducing unnecessary complexity. I'd be happy to discuss this further and collaborate on defining a clear roadmap for the SDK's future. |
I'd like hear this concern about complexity. I think its much simpler to operate in a bunch of micro / smaller packages (although there is acknowledged organic overhead of just doing more than a single package) than to deal with a single monolithic SDK package. You get versioning, the ability to do small breaking changes, run a small subset of tests, and the flexibility to do a lot more things than having to worry about changes to code in other areas of the codebase. Breaking things up is always a tough thing to do, but in fact we really aren't overtly breaking this up for the sake of breaking things up. In fact, I wouldn't really even consider this breaking up the SDK, more about enforcing strong boundaries been modules where that doesn't exist at all currently. To be 100% clear, there are no required or mandatory near future changes. SDK3 can be developed as SDK3 (this PR makes no impact on it). Any of these changes can be back-ported to SDK3 also. Happy to discuss, but the main objective is to organically move things over in time to assist to getting an SDK4.
If you feel there is a pressing need somewhere else, please create a proposal and execute on it and we can compare then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approvable, just had a couple of questions
- name: Integration | ||
working-directory: libs/${{ matrix.package }} | ||
env: | ||
LABELBOX_TEST_API_KEY: ${{ secrets[matrix.api-key] }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where are these matrix attribures (api-key, package, etc) defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See .github/actions/lbox-matrix/index.js
const startingMatrix = [
{
"python-version": "3.8",
"api-key": "STAGING_LABELBOX_API_KEY_2",
"da-test-key": "DA_GCP_LABELBOX_API_KEY"
},
{
"python-version": "3.9",
"api-key": "STAGING_LABELBOX_API_KEY_3",
"da-test-key": "DA_GCP_LABELBOX_API_KEY"
},
{
"python-version": "3.10",
"api-key": "STAGING_LABELBOX_API_KEY_4",
"da-test-key": "DA_GCP_LABELBOX_API_KEY"
},
{
"python-version": "3.11",
"api-key": "STAGING_LABELBOX_API_KEY",
"da-test-key": "DA_GCP_LABELBOX_API_KEY"
},
{
"python-version": "3.12",
"api-key": "STAGING_LABELBOX_API_KEY_5",
"da-test-key": "DA_GCP_LABELBOX_API_KEY"
}
];
Description
https://labelbox.atlassian.net/wiki/spaces/PLT/pages/2658074642/TDD+Labelbox+Python+Repo+Organization
Labelbox lbox example + initial develop.yaml additions
Type of change
Please delete options that are not relevant.
All Submissions
New Feature Submissions
Changes to Core Features